Exclude test environments from Sentry reporting #140
Conversation
derekblank
added
[Type] Enhancement
package.json
Outdated
| "test": "NODE_ENV=test jest", | ||
| "test:watch": "NODE_ENV=test jest --watch", |
There was a problem hiding this comment.
From reviewing documentation and testing locally, defining this explicitly is unnecessary as Jest sets NODE_ENV=test if it is not set to something else.
Jest will set
process.env.NODE_ENVto'test'if it's not set to something else.
That said, I'm fine keeping this here to be explicit, if preferred.
There was a problem hiding this comment.
I see we use process.env.E2E in some other places. Would it make sense to unify this to use one or another everywhere?
From reviewing documentation and testing locally, defining this explicitly is unnecessary as Jest sets NODE_ENV=test if it is not set to something else.
Good points, both. I think it makes sense to leverage the existing process.env.E2E event for E2E tests -- I hadn't yet noted that we were already using it. I'll update both areas and check that the env is being set prior to running E2E builds, as noted in #140 (comment).
There was a problem hiding this comment.
Nice work! From my testing, the latest changes appear to accomplish the desired outcome. Thank you for reducing the error logging noise.
Testing diff
diff --git a/e2e/e2e-helpers.ts b/e2e/e2e-helpers.ts
index feb764d..948e348 100644
--- a/e2e/e2e-helpers.ts
+++ b/e2e/e2e-helpers.ts
@@ -4,6 +4,7 @@ import { _electron as electron } from 'playwright';
export async function launchApp( testEnv: NodeJS.ProcessEnv = {} ) {
// find the latest build in the out directory
const latestBuild = findLatestBuild();
+ console.log( '>>>', { E2E: process.env.E2E, NODE_ENV: process.env.NODE_ENV } );
// parse the packaged Electron app and find paths and other info
const appInfo = parseElectronApp( latestBuild );
diff --git a/src/index.ts b/src/index.ts
index e9ff8b4..d64da96 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -6,6 +6,7 @@ import {
type IpcMainInvokeEvent,
globalShortcut,
Menu,
+ dialog,
} from 'electron';
import path from 'path';
import * as Sentry from '@sentry/electron/main';
@@ -26,6 +27,12 @@ import setupWPServerFiles from './setup-wp-server-files';
import { setupUpdates } from './updates';
import { stopAllServersOnQuit } from './site-server'; // eslint-disable-line import/order
+app.whenReady().then( () => {
+ dialog.showMessageBox( {
+ message: `E2E ${ process.env.E2E } / ENV ${ process.env.NODE_ENV }`,
+ } );
+} );
+
Sentry.init( {
dsn: 'https://97693275b2716fb95048c6d12f4318cf@o248881.ingest.sentry.io/4506612776501248',
debug: true,
Testing outcome
❯ npm run e2e -- app.test.ts
> studio@1.0.2 e2e
> npx playwright install && npx playwright test app.test.ts
Running 1 test using 1 worker
✓ 1 app.test.ts:18:6 › Electron app › should ensure app title is correct. (228ms)
>>> { E2E: undefined, NODE_ENV: undefined }
1 passed (8.3s)
Proposed Changes
Reduce Sentry noise by excluding test environments:
process.env.NODE_ENV !== 'test'to Sentry init configNODE_ENV=testTesting Instructions
Run testing commands like
npm run e2eornpm run testto observe that Sentry events are not reported.Pre-merge Checklist